Fix ineffective jwt+rotate+!revocable combination - #4013
Open
joemahady-comm wants to merge 1 commit into
Open
Conversation
…le combination ai-assisted=yes Co-authored-by: Cursor <cursoragent@cursor.com>
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
joemahady-comm
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enforces a safety constraint around refresh-token rotation: identity zones (and the global token policy) can no longer enable JWT-formatted refresh tokens + rotation unless JWT revocation is enabled, preventing configurations that cannot provide true single-use guarantees.
Changes:
- Added validation in
GeneralIdentityZoneConfigurationValidatorto rejectrefreshTokenRotate=true+refreshTokenFormat=jwtwhenjwtRevocable=false. - Added a startup-time guard in
OauthEndpointBeanConfiguration#uaaTokenPolicyto reject the same invalid property combination for the UAA/global token policy. - Updated/added tests to cover the identity-zone validator behavior and to keep existing refresh-grant tests compatible with the new constraint.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java | Adjusts test properties to satisfy the new constraint when rotation is enabled with JWT refresh tokens. |
| server/src/test/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidatorTests.java | Adds validator tests for invalid/valid combinations of rotate + JWT refresh tokens vs. revocability. |
| server/src/main/java/org/cloudfoundry/identity/uaa/zone/GeneralIdentityZoneConfigurationValidator.java | Enforces identity-zone configuration rejection for rotate+JWT refresh tokens unless revocable. |
| server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/OauthEndpointBeanConfiguration.java | Enforces the same constraint at startup for the global/UAA token policy derived from properties. |
Comments suppressed due to low confidence (2)
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/OauthEndpointBeanConfiguration.java:703
- This introduces a new configuration constraint across
jwt.token.refresh.rotate,jwt.token.refresh.format, andjwt.token.revocable, but the configuration reference forjwt.token.refresh.rotatedoesn’t mention that rotation + JWT refresh tokens requiresjwt.token.revocable=true. Please updatedocs/UAA-Configuration-Reference.mdto document the invalid combination and the required setting.
if (refreshTokenRotate && "jwt".equalsIgnoreCase(refreshTokenFormat) && !jwtRevocable) {
throw new IllegalArgumentException("A token policy cannot have JWT-format refresh tokens with rotation enabled unless they are also revocable.");
}
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/beans/OauthEndpointBeanConfiguration.java:703
- There’s no automated test asserting that the application context fails to start when
jwt.token.refresh.rotate=true,jwt.token.refresh.format=jwt, andjwt.token.revocable=false. Adding anApplicationContextRunner-style test (similar toserver/src/test/java/org/cloudfoundry/identity/uaa/UaaPropertiesTest.java) would lock in this behavior and guard against regressions.
if (refreshTokenRotate && "jwt".equalsIgnoreCase(refreshTokenFormat) && !jwtRevocable) {
throw new IllegalArgumentException("A token policy cannot have JWT-format refresh tokens with rotation enabled unless they are also revocable.");
}
Comment on lines
699
to
705
| bean.setRefreshTokenRotate(refreshTokenRotate); | ||
|
|
||
| if (refreshTokenRotate && "jwt".equalsIgnoreCase(refreshTokenFormat) && !jwtRevocable) { | ||
| throw new IllegalArgumentException("A token policy cannot have JWT-format refresh tokens with rotation enabled unless they are also revocable."); | ||
| } | ||
|
|
||
| return bean; |
Comment on lines
+52
to
+56
| if (tokenPolicy.isRefreshTokenRotate() && | ||
| "jwt".equalsIgnoreCase(tokenPolicy.getRefreshTokenFormat()) && | ||
| !tokenPolicy.isJwtRevocable()) { | ||
| throw new InvalidIdentityZoneConfigurationException("A token policy cannot have JWT-format refresh tokens with rotation enabled unless they are also revocable."); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: Enforced a constraint that prevents identity zones from configuring JWT-formatted refresh tokens with token rotation unless they are also configured to be revocable.
Details: Added checks in GeneralIdentityZoneConfigurationValidator and OauthEndpointBeanConfiguration. If a token policy specifies refreshTokenRotate=true and refreshTokenFormat="jwt" while jwtRevocable=false, it fails validation and throws an exception, ensuring the system enforces true single-use guarantees.